Skip to content

fix(tools): correct CISA KEV detection in get_nvd_data + 55 tests - #176

Open
manus-use wants to merge 1 commit into
mainfrom
fix/nvd-cisa-kev-detection
Open

fix(tools): correct CISA KEV detection in get_nvd_data + 55 tests#176
manus-use wants to merge 1 commit into
mainfrom
fix/nvd-cisa-kev-detection

Conversation

@manus-use

Copy link
Copy Markdown
Owner

Summary

Fixes a real bug in the CISA KEV detection logic within get_nvd_data and adds a comprehensive 55-test suite for the tool function.

The Bug

The existing code checked:

if "cisaExploitAdd" in vulnerability_data.get("cve", {}).get("vulnStatus", ""):

This performs a substring search inside the vulnStatus string (which holds values like "Analyzed", "Modified", "Awaiting Analysis"). The string "cisaExploitAdd" will never appear inside vulnStatus, so CISA KEV information was never detected — even for CVEs actively in the KEV catalog.

The Fix

NVD API v2.0 exposes CISA KEV fields as top-level keys in the cve dictionary object:

{
  "cve": {
    "id": "CVE-2024-1234",
    "vulnStatus": "Analyzed",
    "cisaExploitAdd": "2024-01-20",
    "cisaRequiredAction": "Apply mitigations per vendor instructions.",
    "cisaActionDue": "2024-02-10"
  }
}

The fix checks for the key in the dictionary:

cve_obj = vulnerability_data.get("cve", {})
if "cisaExploitAdd" in cve_obj:

Also adds .get() with empty-string defaults for cisaRequiredAction and cisaActionDue so partial KEV metadata doesn't raise KeyError.

Test Suite (55 tests)

tests/test_get_nvd_data.py — comprehensive coverage of the get_nvd_data tool function:

Category Tests What's covered
TOOL_SPEC metadata 4 name, description, schema, type
Input validation 9 None/empty/int/list/partial/random/missing prefix/no network call/toolUseId
CVE ID normalisation 3 lowercase→uppercase, mixed case, already uppercase
URL construction 2 API v2.0 base, cveId param
Success response 6 status, first vuln entry, CVSS metrics, references, toolUseId, multiple vulns
CISA KEV detection 10 detected, date/action/due extracted, not-in-KEV, always attached, vulnStatus doesn't trigger (old bug), missing cve obj, partial fields
Empty vulnerabilities 4 empty list, missing key, None, error message content
HTTP errors 4 ConnectionError, Timeout, HTTPError, exception text in message
JSON decode error 1 JSONDecodeError handling
Unexpected exceptions 3 RuntimeError caught, TypeError caught, KeyboardInterrupt propagates
log_tool_output_size 6 called on every code path (invalid, success, empty, request error, JSON error, unexpected)
Edge cases 4 long sequence number, no metrics, extra kwargs, missing cve_id key

Test results

1213 passed, 3 deselected, 3 warnings in 24.57s

(Baseline 1158 + 55 new tests, 0 failures)

Duplicate check

Checked all 50 open PRs (#126#175) — no existing open or merged PR fixes this bug or adds tool-level tests for the get_nvd_data function. The closest is:

  • PR feat(tools): NVD rate-limit resilience — retry/back-off + NVD_API_KEY support #91 (merged) — added _nvd_get_with_retry + retry tests, but did NOT test the main get_nvd_data function's response parsing or KEV detection
  • test_nvd_retry.py — existing tests cover the retry helper only; the TestGetNvdDataTool class has just 4 basic tests that don't cover KEV detection, empty responses, JSON errors, or edge cases

The CISA KEV detection logic checked whether "cisaExploitAdd" appeared
inside the vulnStatus *string* (e.g. "Analyzed"), which could never
match.  NVD API v2.0 exposes CISA KEV fields as top-level keys in the
cve object — the fix checks for the key in the dictionary instead.

Also adds graceful .get() defaults for cisaRequiredAction and
cisaActionDue so partial KEV metadata does not raise KeyError.

Includes a comprehensive 55-test suite (tests/test_get_nvd_data.py)
covering: TOOL_SPEC metadata, input validation, CVE ID normalisation,
successful response parsing, CISA KEV detection (the fixed bug),
empty vulnerabilities, HTTP errors, JSON decode errors, unexpected
exceptions, log_tool_output_size invocation, and edge cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant